-
-
Notifications
You must be signed in to change notification settings - Fork 51
update refresh-token-grant errors #140
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Bumps [sinon](https://github.com/sinonjs/sinon) from 12.0.1 to 13.0.1. - [Release notes](https://github.com/sinonjs/sinon/releases) - [Changelog](https://github.com/sinonjs/sinon/blob/master/docs/changelog.md) - [Commits](sinonjs/sinon@v12.0.1...v13.0.1) --- updated-dependencies: - dependency-name: sinon dependency-type: direct:development update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com>
Bumps [mocha](https://github.com/mochajs/mocha) from 9.1.2 to 9.2.2. - [Release notes](https://github.com/mochajs/mocha/releases) - [Changelog](https://github.com/mochajs/mocha/blob/master/CHANGELOG.md) - [Commits](mochajs/mocha@v9.1.2...v9.2.2) --- updated-dependencies: - dependency-name: mocha dependency-type: direct:development update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com>
Bumps [eslint](https://github.com/eslint/eslint) from 8.2.0 to 8.11.0. - [Release notes](https://github.com/eslint/eslint/releases) - [Changelog](https://github.com/eslint/eslint/blob/main/CHANGELOG.md) - [Commits](eslint/eslint@v8.2.0...v8.11.0) --- updated-dependencies: - dependency-name: eslint dependency-type: direct:development update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com>
…yarn/eslint-8.11.0 build(deps-dev): bump eslint from 8.2.0 to 8.11.0
…yarn/mocha-9.2.2 build(deps-dev): bump mocha from 9.1.2 to 9.2.2
…yarn/sinon-13.0.1 build(deps-dev): bump sinon from 12.0.1 to 13.0.1
Two different conditions shouldnt throw the same error
@chrisfranko thank you for contributing to this project. Please complete the sections from the issue template, resolve the merge confliczts and update the tests accordingly. We use Since this would be a breaking change we first need to discuss this with respect to the RFC. @Uzlopak @HappyZombies @jorenvandeweyer |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would change them like I remarked.
Also unit tests should be adapted.
Co-authored-by: Uzlopak <aras.abbasi@googlemail.com>
Co-authored-by: Uzlopak <aras.abbasi@googlemail.com>
@jankapunkt how could this be a breaking change? It's just changing the error message. Though someone could be testing for this particular error message, but eh, that would be wrong of them to test. |
@HappyZombies sorry I misread it. It's only the error message that changed while I thought it's the error class that changed. From that point it should be a good improvement. The package.json and package-lock.json should be restored to the previous values and the yarn.lock should be removed then it's good to go imo. |
Done! |
@chrisfranko dope thanks, but can we remove the sinon update and remove the lock file changes. Then we will approve and merge |
Yea man I thought I did that? |
But honestly im not entirely sure how to remove the sinon update because those all came from master and I didnt manually do them. However! It might just be easier for me to push a new pr to the dev branch instead. |
Two different conditions shouldnt throw the same error
Summary
The main problem is various errors were identical in the Refresh Token Grant file. While working on an express/mongodb adapter I kept erroring out at the client check. Well the client check error is the same error that exist in like 5 other places. Just changing the verbiage alone would help in diagnosing the problem.
Linked issue(s)
Involved parts of the project
[https://github.com/node-oauth/node-oauth2-server/blob/master/lib/grant-types/refresh-token-grant-type.js](Refresh Grant Type)
Added tests?
si
OAuth2 standard
Its just the error verbiage the logic remains untouched.
Reproduction
No new features just changing some text.